update version all dependencies, migrate to async #17
Conversation
crbernal
commented
Mar 22, 2022
crbernal
commented
- Packages -> update version of dependencies & delete sync
- Update to async all the functions
There was a problem hiding this comment.
In the tests I've only mentioned in one of them what I see weird (using await where it looks to me like it's not needed) but, it's present in most of them.
The other thing that I'm very curious about is if the $(selector) is async, but I've seen now in the docs (https://webdriver.io/docs/api/element/$) that it is :) Interesting!
| @@ -21,5 +21,5 @@ export default (action, type, selector) => { | |||
|
|
|||
| checkIfElementExists(selector2); | |||
|
|
|||
| $(selector2)[method](); | |||
| await selector2[method](); | |||
There was a problem hiding this comment.
Is this one correct? Souldn't it be await $(selector2)[method](); like it used to be?
| @@ -5,22 +5,22 @@ | |||
| * @param {String} element Element selector | |||
| * @param {String} element Element selector | |||
| */ | |||
| module.exports = (previousElement, element) => { | |||
| module.exports = (previousElement, element) => { | |||
There was a problem hiding this comment.
This should be async as well, right? Also, why is this export different than the rest of actions?
|
|
||
| var value = await $(previousElement)[command](); |
There was a problem hiding this comment.
Non-relevant, but we should avoid var :)
There was a problem hiding this comment.
Cool, I'm going to replace all the var!
| @@ -9,7 +9,7 @@ import setInputField from './setInputField'; | |||
| */ | |||
| module.exports = (method, element) => { | |||
|
|
|||
| var date = Date.now().toString(); | |||
| var date = await Date.now().toString(); | |||
There was a problem hiding this comment.
Why? Date is synchronous in JS, did you have any problem that forced you to do this?
| @@ -8,7 +8,7 @@ import setInputField from "./setInputField"; | |||
| */ | |||
| module.exports = (method, element) => { | |||
|
|
|||
| var date = Date.now().toString(); | |||
| var date = await Date.now().toString(); | |||
| @@ -38,8 +38,8 @@ export default (elementType, selector, falseCase) => { | |||
| } | |||
|
|
|||
| if (boolFalseCase) { | |||
| expect(text).toBe(''); | |||
| await expect(text).toBe(''); | |||
There was a problem hiding this comment.
I have a question here... and I see it repeats in other texts. text is a value that you obtain from an await already on line 32. As far as I know, expect().toBe() is a synchronous check, as you already have the value you want to check. It would be different if it was something like
await expect($(selector)[command]()).toBe('');
because then that line has some asynchronous stuff going on. But if the async "wait" happened already, I don't think you need it. Unless I'm missing something about the framework 🤔
| @@ -36,9 +36,9 @@ export default (elementType, selector, falseCase, expectedText) => { | |||
| * The text of the element | |||
| * @type {String} | |||
| */ | |||
| const elem = $(selector); | |||
| const elem = await $(selector); | |||
There was a problem hiding this comment.
I've seen this a couple of times... Is this requested by the framework? I mean, are the selectors supposed to be asynchronous? Or are just the methods called on the resulting elem from the selector the ones that are async?
| @@ -106,7 +106,7 @@ exports.config = { | |||
| baseUrl: 'http://localhost:3000/#/login', | |||
| // | |||
| // Default timeout for all waitFor* commands. | |||
| waitforTimeout: 19000, | |||
| waitforTimeout: 99999, | |||
There was a problem hiding this comment.
Are we sure we want this in the default boilerplate? Shouldn't this be something that we override per project in case it's needed, but should be a reasonable number? Honestly more that 20 seconds shouldn't be the "normal" for the tests to blow, right? And you can always override the value on a per-project basis in case it's needed, no? What do you think?
| './node_modules/qustodio-itui/src/steps/when.js', | ||
| './node_modules/qustodio-itui/src/steps/then.js', | ||
| require: [ | ||
| './src/steps/**/*.feature', |
There was a problem hiding this comment.
This one I don't fully understand it. We're removing the steps (why?) and setting the path where we expect to find the features... That should be a responsibility of the project using itui, but not prescriptive from itui directly, no? Maybe there's some other intent here that I'm not seeing, just asking to uderstand here :)
